Accept the zoom in hotkey when shift is held - #1268
Open
karpovantonme wants to merge 1 commit into
Open
Conversation
On most keyboard layouts '+' is typed as shift + '=', so the key press reaches the app with shift among the modifiers. The subscription matched the modifier set by equality, so ctrl + '+' fell through to the catch-all and no zoom happened. ctrl + '-' was unaffected since '-' needs no shift. Accept '=' as a zoom in key too, the way browsers do, and handle the command + shift combination. The hotkey mapping moves into its own function so it can be covered by tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ctrl++does not zoom in on layouts where+is typed asshift+=. That covers US and UK ANSI and most others. On layouts where+has its own key (Italian, German) it works, which is probably why it went unnoticed.The subscription in
src/gui/sniffer.rsmatches the whole modifier set by equality:Modifiersis abitflagsstruct, soModifiers::COMMANDas a pattern means "command and nothing else". To produce the logical key+the user has to hold shift, andiced_winit::conversion::modifierspasses shift straight through:So the event arrives with
COMMAND | SHIFT, matches neitherModifiers::COMMANDnorModifiers::SHIFT, and falls into the catch-all. Zoom out is unaffected because-needs no shift, which is why only one half of the shortcut is broken.The shortcut came from #554, where it was asked for as an accessibility feature, so it seemed worth fixing properly.
Change
command+shift++=as a zoom in key too, without shift. That is what browsers do, and it gives people a shift-free way inhotkey_message(), so it can be tested. Behaviour outside the zoom keys is unchanged, I kept the match arms in the same orderChecks
cargo test- 178 passed, 0 failedcargo fmt --check- cleancargo clippy --all-targets- 57 warnings before the change and 57 aftertest_zoom_hotkeysfails onmainwith the fix reverted and passes with it, so it guards the actual bugIf you would rather keep the subscription as it was, the one-line version is to add
"="next to"+"in the existing arm. That fixes the shift-free path but leavesshift+=dead. Happy to cut it down if you prefer.